Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: reduce autoconf input files from 32 to 2 #5219

Merged
merged 3 commits into from
Jun 30, 2022

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Jun 25, 2022

Configure summary: autoconf essentially only parses configure.ac and
generates the configure script (that is, the "./configure" shell
script). The latter is what actually checks what is available on the
system and internally sets the value of the output variables. It then,
for every filename foo in AC_CONFIG_FILES (and for every output variable
name BAR in AC_SUBST), reads foo.in, replaces every occurrence of
@BAR@ with the value of the shell variable $BAR and generates the
file foo from the result. After this, configure is finished and make
could be executed to start the build.

Now that (as of #5140) all output variables are only defined on
config.mk.in and on config.sh.in, there is no need to generate any
makefile nor any other mkfile or shell script at configure time. So
rename every "Makefile.in" to "Makefile", mkdeb.sh.in to mkdeb.sh,
src/common.mk.in to src/common.mk and leave just config.mk and config.sh
as the files to be generated at configure time.

This allows editing and committing all makefiles directly, without
potentially having to run ./configure in between.

Commands used to rename the makefiles:

$ git ls-files -z -- '*Makefile.in' | xargs -0 -I '{}' sh -c \
  "git mv '{}' \"\$(dirname '{}')/Makefile\""

Additionally, from my (rudimentary) testing, this commit reduces the
time it takes to run ./configure by about 20~25% compared to commit
72ece92 ("Transmission fixes: drop private-lib (#5213)", 2022-06-22).
Environment: dash 0.5.11.5-1, gcc 12.1.0-2, Artix Linux, ext4 on an HDD.

Commands used for benchmarking each commit:

$ : >time_configure && ./configure && make distclean &&
  for i in $(seq 1 10); do
  { time -p ./configure; } 2>>time_configure; done
$ grep real time_configure |
  awk '{ total += $2 } END { print total/NR }'

@kmk3
Copy link
Collaborator Author

kmk3 commented Jun 25, 2022

To clarify, here is a summary of how the output files are generated from the
input files. Using the following format:

  • input-file(s) -> generator-command -> output-file(s)

Firstly, the configure script is still generated in the same way as always:

  • configure.ac -> autoconf -> configure

As of firejail 0.9.70 (that is, prior to #5140), this is what happens:

  • Makefile.in (x28)
  • mkdeb.sh.in
  • src/common.mk.in

-> ./configure ->

  • Makefile (x28)
  • mkdeb.sh
  • src/common.mk

That is, every makefile is generated, regardless of whether it depends on any
output variable (such as @CFLAGS@) or not.


With #5140 and this PR:

  • config.mk.in
  • config.sh.in

-> ./configure ->

  • config.mk
  • config.sh

Which means that the output variables (such as @NAME@ and @VERSION@) are
all kept on language-specific include files, which allows all makefiles to
include config.mk (and all shell scripts to include config.sh) without having
to care about where config.mk (or config.sh) comes from (that is, whether it is
a normal file or a generated file).

kmk3 added 3 commits June 30, 2022 05:30
Configure summary: autoconf essentially only parses configure.ac and
generates the configure script (that is, the "./configure" shell
script).  The latter is what actually checks what is available on the
system and internally sets the value of the output variables.  It then,
for every filename foo in AC_CONFIG_FILES (and for every output variable
name BAR in AC_SUBST), reads foo.in, replaces every occurrence of
`@BAR@` with the value of the shell variable `$BAR` and generates the
file foo from the result.  After this, configure is finished and `make`
could be executed to start the build.

Now that (as of netblue30#5140) all output variables are only defined on
config.mk.in and on config.sh.in, there is no need to generate any
makefile nor any other mkfile or shell script at configure time.  So
rename every "Makefile.in" to "Makefile", mkdeb.sh.in to mkdeb.sh,
src/common.mk.in to src/common.mk and leave just config.mk and config.sh
as the files to be generated at configure time.

This allows editing and committing all makefiles directly, without
potentially having to run ./configure in between.

Commands used to rename the makefiles:

    $ git ls-files -z -- '*Makefile.in' | xargs -0 -I '{}' sh -c \
      "git mv '{}' \"\$(dirname '{}')/Makefile\""

Additionally, from my (rudimentary) testing, this commit reduces the
time it takes to run ./configure by about 20~25% compared to commit
72ece92 ("Transmission fixes: drop private-lib (netblue30#5213)", 2022-06-22).
Environment: dash 0.5.11.5-1, gcc 12.1.0-2, Artix Linux, ext4 on an HDD.

Commands used for benchmarking each commit:

    $ : >time_configure && ./configure && make distclean &&
      for i in $(seq 1 10); do
      { time -p ./configure; } 2>>time_configure; done
    $ grep real time_configure |
      awk '{ total += $2 } END { print total/NR }'
This allows running `make clean` and `make distclean` (and possibly
others) without having to run ./configure beforehand.

Note that some packaging-related targets still depend on the existence
of generated files.  For example:

* dist: config.mk
* deb: config.sh

Commands used to search and replace:

    $ git grep -Elz 'include *([^ ]*/)?config.mk' | xargs -0 -I '{}' \
      sh -c "printf '%s\n' \
      \"\$(sed -E 's|^include *(([^ ]*/)?config.mk)|-include \1|' '{}')\" >'{}'"

Relates to netblue30#5140.
With the previous commit ("makefiles: stop failing when config.mk does
not exist", 2022-06-23), make will not immediately fail when trying to
build a target without having the proper compile-time flags (which are
defined on common.mk).

For example, when running the command below:

    make distclean && make

It will throw an error only after (mis-)compiling multiple objects.

So add a dependency on config.mk on every target that uses output
variables (such as @name@ / $(NAME)) on its recipe.  And add a
dependency on config.sh on targets that call shell scripts that use
output variables (such as @name@ / $NAME).  Also, add a recipe for
config.mk / config.sh telling to run ./configure, to make it a bit more
obvious just in case.

With this commit, make will abort earlier, by detecting that the
config.mk / config.sh dependency does not exist.  This happens before
trying to execute the recipe.

This also makes the dependencies more accurate, since if config.mk
(which defines some CFLAGS) is changed, the CFLAGS may also have
changed, so a target that uses CFLAGS should probably be considered out
of date in this case anyway.

Relates to netblue30#5140.
@kmk3 kmk3 force-pushed the build-reduce-config-files branch from 02510ea to e21637c Compare June 30, 2022 08:37
@kmk3
Copy link
Collaborator Author

kmk3 commented Jun 30, 2022

Force-pushed to add more information to the first commit message, as I noticed
that using bash + clang almost doubles the time it takes to run ./configure
(especially because of clang), compared to dash + gcc.

@netblue30 netblue30 merged commit 70aaf6f into netblue30:master Jun 30, 2022
@netblue30
Copy link
Owner

all merged, thanks!

@kmk3 kmk3 deleted the build-reduce-config-files branch June 30, 2022 13:50
kmk3 added a commit to kmk3/firejail that referenced this pull request Jul 12, 2022
The following leverages the fact that when using a normal merge (as
opposed to "rebase and merge" or "squash and merge") on GitHub, the pull
request number is put in the commit message title and the title of the
PR is added to the commit message body.

Commands used to find and print the items for the RELNOTES:

    $ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
      sed -E -n 's/Merge pull request (#[0-9]+) from [^ ]+ (.*)/  * \2 (\1)/p'
      * build: deduplicate configure-time vars into new config files (netblue30#5140)
      * build: fix file mode of shell scripts (644 -> 755) (netblue30#5206)
      * build: reduce autoconf input files from 32 to 2 (netblue30#5219)

Commands used to generate the message below:

    $ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
      sed -E -n 's/Merge pull request (#[0-9]+).*/\1/p' | sort | tr '\n' ' ' |
      sed -E 's/^(.*) /Relates to \1./'
    Relates to netblue30#5140 netblue30#5206 netblue30#5219.

Relates to netblue30#5140 netblue30#5206 netblue30#5219.
kmk3 added a commit that referenced this pull request Jul 12, 2022
The following leverages the fact that when using a normal merge (as
opposed to "rebase and merge" or "squash and merge") on GitHub, the pull
request number is put in the commit message title and the title of the
PR is added to the commit message body.

Commands used to find and print the items for the RELNOTES:

    $ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
      sed -E -n 's/Merge pull request (#[0-9]+) from [^ ]+ (.*)/  * \2 (\1)/p'
      * build: deduplicate configure-time vars into new config files (#5140)
      * build: fix file mode of shell scripts (644 -> 755) (#5206)
      * build: reduce autoconf input files from 32 to 2 (#5219)

Commands used to generate the message below:

    $ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
      sed -E -n 's/Merge pull request (#[0-9]+).*/\1/p' | sort | tr '\n' ' ' |
      sed -E 's/^(.*) /Relates to \1./'
    Relates to #5140 #5206 #5219.

Relates to #5140 #5206 #5219.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 28, 2023
And also add an "error: " prefix, to make the output clearer.

Before:

    $ rm -f config.mk; make config.mk
    printf 'run ./configure to generate %s\n' "config.mk" >&2
    run ./configure to generate config.mk
    false
    make: *** No rule to make target 'config.mk'.  Stop.

After:

    $ rm -f config.mk; make config.mk
    error: run ./configure to generate config.mk
    make: *** No rule to make target 'config.mk'.  Stop.

This amends commit e21637c ("makefiles: add generated files as
dependencies", 2022-06-23) / PR netblue30#5219.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

3 participants